Skip to content

Conversation

@jaydeepkumar1984
Copy link
Contributor

@jaydeepkumar1984 jaydeepkumar1984 commented Jan 21, 2024

As part of CASSANDRA-17248 (in C* 4.0.2), the following code was introduced in QueryProcessor.java

// Make sure the missing one is going to be eventually re-prepared                
evictPrepared(hashWithKeyspace);                
evictPrepared(hashWithoutKeyspace); 

This code could very well create a race condition between two calls of QueryProcessor::prepare call in which one thread is adding and another thread is silently removing from the cache. Imagine there are thousands of threads calling the API, and then it might be possible for one thread to update the cache and another thread to remove it.

If we look at the code of Cassandra 3.0.25, then such eviction was not present. Hence, this is a regression for the post 4.0.2 version of Cassandra. To fix this issue, we should not evict the cache entries, i.e., the above-mentioned code path introduced since C* 4.0.2 is no longer required on trunk/5.0, as discussed in the ticket.

For the 4.0 branch, we will keep the eviction logic but optimize it further to address CASSANDRA-17401, as it is essential for folks on pre-4.0.1 trying to upgrade to the latest 4.0 version.

Cassandra-17401

Reproducing this is extremely tricky because it totally depends on the timing of the multiple threads, here is the pull request that reproduces it: #3058

@belliottsmith belliottsmith force-pushed the trunk branch 2 times, most recently from df3eb40 to 54e39a9 Compare July 23, 2025 11:19
Copy link
Contributor

@ifesdjeen ifesdjeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, assuming
a) this patch is only committed to trunk
b) 5.0 / 5.1 or whatever version we consider to be an upgrade path for post-15252 still has this logic to preclude incorrect statements being cached during upgrade

@jaydeepkumar1984
Copy link
Contributor Author

jaydeepkumar1984 commented Jan 13, 2026

Thanks a lot @ifesdjeen for the review! Sure, I will commit this patch to trunk only.
I will also make sure the upgrade path has this version in a separate PR.

@jaydeepkumar1984
Copy link
Contributor Author

jaydeepkumar1984 commented Jan 19, 2026

I have just committed this approved PR to trunk.
a06df09 & aa4da83

Here is the PR for 5.0 jaydeepkumar1984#67 - please review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants